Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Endpoint][EPM] Retrieve Index Pattern from Ingest Manager #63016

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Apr 8, 2020

Important Change

When having the endpoint send metadata use the index metrics-endpoint-default-1.

Summary

I'm going to leave this PR in draft until the the endpoint-ingest dependency PR is merged here: #62871

Once that PR is merged this one's number of changed files will go down.

This PR exposes functionality in the Ingest Manager to construct an index pattern based on the contents of a package. The endpoint app will rely on the ingest manager to retrieve index patterns for querying elasticsearch. The advantage of using the package to construct the index pattern is it isolates the logic for defining the index patterns to only the endpoint package so it only needs to be maintained there. The downside is a request must be made to access a saved object when the index pattern is retrieved for each time that index pattern is needed. This is done once per request when needed. If a route handler makes multiple queries, it only retrieves the pattern once. I spoke with the Kibana team about the extra overhead of using saved objects and they recommended not worrying about it for now. If it becomes an issue we can implement a cache within the endpoint app.

Notable Changes

Ingest Manager

  • Modified the structure of the saved objects used to store information about installed packages
    • I added a patterns field which is a map of dataset to constructed index pattern (e.g. events -> events-endpoint-*)
  • Added a setup contract that allows other plugins server to make calls into the ingest manager's server without using an http handler
  • Created a helper service in the api_integration test directory to handle setting up the ingest manager before the endpoint api_integration tests. This is needed because even though the endpoint app depends on the ingest manager, since the UIs are not used it's setup function will not be called

Endpoint

  • Removed almost all references to hardcoded index patterns, there is still one place within the alert handlers that remains because we need to encode the index of alerts in the id field when returning them to the UI. I'll make that change in a follow up PR once this merged.
  • Added the ingest manager as a dependency for the endpoint app
  • Added a class for communicating with the ingest manager
  • Added a API route for retrieving an index pattern so the alert list view can set the index pattern accordingly
  • Added api_integration tests for the scenario when the ingest manager is not setup

Testing

If you'd like to test this PR you'll need to add a couple flags to your kibana.dev.yml file:

xpack.ingestManager.enabled: true
#xpack.ingestManager.epm.registryUrl: 'http://127.0.0.1:8080'
xpack.ingestManager.epm.enabled: true
xpack.ingestManager.fleet.enabled: true

The ingest manager will pull the packages from the default package registry URL. If you did want to run your own package registry you can point ingest manager to it using the xpack.ingestManager.epm.registryUrl (more info here: https://github.com/elastic/package-registry and https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/README.md)

For the endpoint app you can try populating elasticsearch with events from the alerts es_archive and make sure you can still see them in the alert list view:

image

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Endpoint Elastic Endpoint feature v7.8.0 labels Apr 8, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍


async getIndexPattern(ctx: RequestHandlerContext, datasetPath: string) {
try {
const pattern = await this.service.getESIndexPattern(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should (can?) any of these call be cached?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I haven't created an issue for it yet but we'll create a cache so we don't have to do so many calls to the saved objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,11 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious why you had to create this typescript definition file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it wasn't necessary, it seemed like most test directories had their own but I think it's only useful if we define our own services and page_objects.

Copy link
Contributor

@charlie-pichette charlie-pichette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes for tests look good. 👍

@oatkiller
Copy link
Contributor

Created a helper service in the api_integration test directory to handle setting up the ingest manager before the endpoint api_integration tests. This is needed because even though the endpoint app depends on the ingest manager, since the UIs are not used it's setup function will not be called

@jonathan-buttner Can you explain this a little? The Endpoint client code is responsible for calling the ingest server side setup function? I know you mentioned it to me, but I can't remember what the reasoning was.

return await this.getIndexPattern(ctx, EndpointAppConstants.EVENT_DATASET);
}

async getMetadataIndexPattern(ctx: RequestHandlerContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind adding a quick doc comment to public methods? maybe

/**
 * fetch and return the index pattern used to store Endpoint host metadata.
 */

I think these types of comments will help a lot, esp. considering the size of the org

};
}

export function registerIndexPatternRoute(router: IRouter, endpointAppContext: EndpointAppContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this for the alert list feature? if so, thoughts on moving it to the alerting routes module?

Copy link
Contributor Author

@jonathan-buttner jonathan-buttner Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's currently only used for the alert list. The alert list uses an index pattern when doing time based filtering I think. I think having an index pattern is required when doing that.

I could foresee other pages (Host maybe?) needing an index pattern if they want to do any time based filtering too. If you'd prefer it in the alert list for now until it's needed in the future, that's fine too though.

@@ -34,7 +39,7 @@ describe('query builder', () => {
aggs: {
total: {
cardinality: {
field: 'host.id.keyword',
field: 'host.id',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im a bit of a noob about this. can you explain this change? is this just keeping us in sync w/ latest mappings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's because the mapping that was being used for metadata was based off a dynamically generated mapping created by default by elasticsearch when no mapping exists. Now that we have solidified and generated a mapping using ECS we want to use that. With the new mapping the field keyword no longer exists.

https://www.elastic.co/blog/strings-are-dead-long-live-strings

"successful" : 1,
"skipped" : 0,
"failed" : 0
"took": 343,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain the change here? also, anyone know why this is large and not gzipped?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathan-buttner This will be on me to fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oatkiller most of the changes are just spacing and renaming the index being used:

image

my vs code default formatter is removing the spaces like these, I can revert them if you prefer:

image

I believe this is not gzipped because it's directly imported in typescript code here:
https://github.com/elastic/kibana/pull/63016/files/2829689ae9674970d57644212863734d34d2cd5b#diff-b1e4abedd6d2153cf169796ebae56a93R27
image

@nnamdifrankie yeah if you could fix it that'd be great!

): Promise<string | undefined>;
}

export class ESIndexPatternSavedObjectService implements ESIndexPatternService {
Copy link
Contributor

@oatkiller oatkiller Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side note, classes in ts are also interfaces, so ESIndexPatternSavedObjectService can be used exactly the same as ESIndexPatternService Using a class as an interface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool! Didn't realize that.

export default function({ getService }: FtrProviderContext) {
const supertest = getService('supertest');

describe('Endpoint index pattern API', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned this earlier, but what is the purpose of exposing these APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it doesn't have to be, it's only used by the alert list but could be useful if other UIs need to do time based filtering of a list.

@jonathan-buttner
Copy link
Contributor Author

Created a helper service in the api_integration test directory to handle setting up the ingest manager before the endpoint api_integration tests. This is needed because even though the endpoint app depends on the ingest manager, since the UIs are not used it's setup function will not be called

@jonathan-buttner Can you explain this a little? The Endpoint client code is responsible for calling the ingest server side setup function? I know you mentioned it to me, but I can't remember what the reasoning was.

@oatkiller yeah, the Ingest Manager plugin will handle calling the setup routine in its front end plugin's start function. This means that initialization occurs when the Kibana UI is loaded (you don't have to open any particular app, just log in to Kibana). The Ingest manager communicates the status of the initialization via the plugin's contract using a boolean. So the Endpoint app does not call setup. For api_integration tests there is no UI to load so the initialization does not occur. The endpoint api_integration tests need the index pattern to query elasticsearch and still need to get it from the Ingest manager but can only retrieve it after the ingest manager is initialized. To force the initialization to occur during api_integration tests, I created a service that handles calling the setup API route: https://github.com/jonathan-buttner/kibana/blob/endpoint-ingest-index-pattern/x-pack/test/api_integration/services/ingest_manager.ts#L22

That service is then used before our endpoint api_integration tests run here: https://github.com/jonathan-buttner/kibana/blob/endpoint-ingest-index-pattern/x-pack/test/api_integration/apis/endpoint/index.ts#L17

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 17, 2020
* master: (40 commits)
  [APM]Upgrade apm-rum agent to latest version to fix full page reload (elastic#63723)
  add deprecation warning for legacy 3rd party plugins (elastic#62401)
  Migrate timelion vis (elastic#62819)
  Replacebad scope link with actual values (elastic#63444)
  Index pattern management UI -> TypeScript and New Platform Ready (create_index_pattern_wizard) (elastic#63111)
  [SIEM] Threat hunting enhancements: Filter for/out value, Show top field, Copy to Clipboard, Draggable chart legends (elastic#61207)
  [Maps] fix term join agg key collision (elastic#63324)
  [Ingest] Fix agent config key sorting (elastic#63488)
  [Monitoring] Fixed server response errors (elastic#63181)
  update elastic charts to 18.3.0 (elastic#63732)
  Start services (elastic#63720)
  [APM] Encode spaces when creating ML job (elastic#63683)
  Uptime 7.7 docs (elastic#62228)
  [DOCS] Updates remote cluster and ccr docs (elastic#63517)
  [Maps] Add 3rd party vector tile support (elastic#62084)
  [Endpoint][EPM] Retrieve Index Pattern from Ingest Manager (elastic#63016)
  [Endpoint] Host Details Policy Response Panel (elastic#63518)
  [Uptime] Certificate expiration threshold settings (elastic#63682)
  Refactor saved object types to use `namespaceType` (elastic#63217)
  [SIEM][CASE] Create comments sequentially (elastic#63692)
  ...
jonathan-buttner added a commit that referenced this pull request Apr 17, 2020
…63767)

* Endpoint successfully depending on ingest manager to initialize

* Moving the endpoint functional tests to their own directory to avoid enabling ingest in the base tests

* Removing page objects and other endpoint fields from base functional

* Updating code owners with new functional location

* Adding index pattern functionality

* Missed a file

* Pointing resolver tests at endpoint functional tests

* Pointing space tests at the endpoint functional directory

* Adding ingest service to do setup and tests for 500s

* Correcting services path

* Adding jest test names

* Updating es archives with the correct mapping and index names

* Fixing import error

* Adding resolver tests to code owners

* enabling epm flag for functional tests

* adding correct tag to test

* Removing the version information and unneeded xsrf

* Addressing endpoint index pattern feedback

* Removing unused import

* Renaming index pattern to es index pattern

* Fixing missed index pattern calls

* Removing unused import

* Fixing type error

* Moving es_index_pattern outside of installed and fixing function name

* Keeping the event index the same for now

* Wrapping index pattern await in try catch

* Address PR feedback, adding comments

Co-authored-by: Elastic Machine <[email protected]>
@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Endpoint Elastic Endpoint feature Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.